feat(open-knowledge): ask why on uninstall — churn feedback survey (desktop + CLI) (#2849)#775
Merged
Merged
Conversation
…esktop + CLI) (#2849)
* [US-001] add shared uninstall feedback reasons taxonomy to core
The desktop uninstall window and the `ok uninstall` CLI prompt both need
the same "why are you leaving?" reason list. Declaring it once in core
keeps the two surfaces from drifting.
The slugs are the contract: they travel to /api/feedback inside the
opaque `reasons` array and are what churn tickets get grouped by, so the
test pins the exact set and order. Labels are display-only plain English
(neither consumer is Lingui-wired) and stay free to reword.
* [US-002] add plain-Node uninstall feedback submit helper to core
* [US-003] add filterable uninstall Linear label to feedback intake
Uninstall feedback tickets now carry LINEAR_UNINSTALL_FEEDBACK_LABEL_IDS on
top of the shared LINEAR_FEEDBACK_LABEL_IDS, so churn feedback is filterable in
Linear on its own rather than by title search. General feedback is unchanged,
and the new var is optional so the code can ship ahead of creating the label and
setting its id in Vercel env.
The union is a Set so an operator who lists the shared id in both vars cannot
make issueCreate receive the same label id twice, and parseLabelIds already
trims and drops empties so no empty-string id is ever sent. The /api/feedback
request schema is untouched.
Covers the three label branches in route.test.ts against the real issueCreate
GraphQL input, plus the both-unset case (no labelIds key at all).
* [US-004] add desktop uninstall feedback window builder and result parser
Adds buildDesktopUninstallFeedbackHtml() and parseDesktopUninstallFeedbackUrl()
to desktop-uninstall.ts: the optional churn survey shown between the confirm
surface and cleanup, in the same inline-HTML native-dialog vocabulary as the
sibling picker and notice windows.
The screen renders the shared 7-reason taxonomy as a single-select radio group
plus an optional note and an opt-in email field. It has no Escape handler and
no backdrop, so the only exits are the two buttons or a deliberate window
close. Answers return to main on the existing ok-desktop-uninstall: scheme
under feedback-skip / feedback-send hosts.
Core gains isUninstallFeedbackReason() so the parser can re-narrow the slug
that arrives back as an arbitrary string; an off-taxonomy slug is dropped
while the note and email survive, and both are clamped to the intake's field
limits so an oversized answer cannot reject the whole ticket.
The send button submits the form rather than handling a click, letting the
browser's own email validation run first; the email input stays disabled until
the opt-in is checked, since a hidden-but-validatable field makes the browser
refuse the submit with nothing visible to fix.
* [US-005] show the uninstall feedback screen between confirm and cleanup
Extracts the pre-cleanup flow decisions into the Electron-free
desktop-uninstall module so they are unit-testable: confirmDesktopUninstall
runs the project picker or the plain confirm notice, then awaits the feedback
step after either branch before reporting proceed. runDesktopUninstallFeedbackStep
skips the POST when the user left nothing, awaits the bounded submit so it
flushes before app.quit tears the process down, and reports failures as an
outcome instead of throwing.
Main wires a feedback window shaped like the project picker, except closing it
or failing to load resolves an empty answer set and continues the uninstall.
Reusing the picker's close-means-cancel mapping would have turned an optional
question into a second, hidden cancel gate for a decision the user already made.
* [US-006] add the interactive uninstall feedback prompt to the CLI
After the existing destructive confirm and before removal, `ok uninstall`
now offers the shared 7-reason churn survey plus an optional note and an
optional follow-up email, then POSTs it best-effort with source
'cli_uninstall'.
The prompt fires only when stdin and stdout are both terminals and
neither --yes nor --json is set. Gating on stdout alone would still
prompt a caller that piped only its input, and the desktop cleanup
script shells out `ok uninstall --yes`, where a prompt would hang a
detached process.
Nothing in the survey can abort the removal: the send is awaited so it
flushes before the process exits, but an interrupted prompt, a closed
stdin, or a failed transport all fall through to the removal.
@inquirer/input is not a dependency here, so the note and email prompts
use node:readline/promises the way confirmDestructive already does. All
survey prompts render on stderr so stdout stays the removal report.
* [ship] add changeset for uninstall feedback form
* docs(open-knowledge): uninstall feedback form spec + evidence
* feat(open-knowledge): ask uninstall feedback after removal, only on success
Reorder desktop + CLI so feedback is asked after a successful removal
(not before): only surveys users whose uninstall completed, and removes
the answer-before-doing friction. Desktop decision logic in the new
runDesktopUninstallOutcomeStep; CLI gated on outcome.failed.length === 0.
D13/D14/FR3 unchanged. (spec D15)
* feat(open-knowledge): add email example placeholder to uninstall feedback
Format-hint parity with the in-app feedback form's example, while keeping
the uninstall form's real <label> (the in-app form uses placeholder-as-label,
which the uninstall screen deliberately avoids).
* chore(open-knowledge): address local-review findings
Hardening from the local AI review convergence pass on the uninstall
feedback surface: tighten the feedback window CSP (form-action/base-uri),
clamp the result-channel note/email to the intake field limits, drop an
out-of-taxonomy reason slug on parse, add a retry guard + shared deadline
to the submit helper, and a11y/doc fixes (aria-describedby, SPEC §4
corrigendum). Review verdict: APPROVE, 0 findings remaining.
* feat(open-knowledge): redesign uninstall completion screen as a checklist
The 'OpenKnowledge files were removed' notice was wall-of-text-y and buried
the one action (move-to-Trash) in recap prose. Restructure it as a done/pending
checklist so the two done items are glanceable reassurance and the pending
'Move to Trash' item is the obvious next step, and turn the raw cleanup-log
path into a subtle 'Cleanup log' link that reveals it in Finder (via an
intercepted reveal-log navigation; the path never enters the HTML). Primary
button relabeled 'Reveal in Finder'. (spec D16)
* feat(open-knowledge): tighten uninstall feedback + completion copy
Feedback screen: drop the now-redundant 'content is kept' reassurance line
(the completion checklist covers it) and shorten 'Continue without feedback'
to 'Skip'. Completion screen: clearer move-to-Trash copy ('drag it to the
Trash' rather than the vague 'drag it away').
* feat(open-knowledge): clarify move-to-Trash copy + align the failure-path final step
Fix the agency in the completion move-to-Trash line ('so you can drag it to the
Trash' — the app reveals + quits, you drag), and give the failure-path 'One more
step' screen the same copy + 'Reveal in Finder' button as the success screen
(it was left on the old 'Continue' wording).
* refactor(open-knowledge): share one type scale across uninstall windows
The four uninstall windows (picker, feedback, progress, notices) were each a
self-contained inline-HTML doc with a duplicated <style>, so the type scale had
drifted (a 12px header line on feedback, an 18px heading on progress vs 20px
elsewhere). Extract a shared DESKTOP_UNINSTALL_BASE_CSS (fonts, heading size,
muted colour) that every screen references, so text sizing has one source of
truth. Also remove the now-redundant subtext line under the feedback heading
(the completion checklist already reassures 'Kept your content').
* fix(open-knowledge): drop the divider rule under the feedback heading
The feedback window's header carried a border-bottom divider that, with the
subtext line now gone, left a bare horizontal rule floating under 'Thanks for
giving OpenKnowledge a try.' Remove the border and tighten the header padding so
the heading sits directly above the form.
* test(open-knowledge): dev-only non-destructive uninstall UI preview
Add OK_UNINSTALL_UI_PREVIEW (dev builds only) to walk the real uninstall
windows — project picker, progress, feedback survey, completion/failure
notices — without removing anything or trashing/quitting the app. Reuses the
exact window builders and the runDesktopUninstallOutcomeStep spine; cleanup is
stubbed to the requested success|failure outcome. Feedback POSTs only when a
non-production OK_FEEDBACK_INTAKE_ORIGIN is set, so a preview can never reach
production intake. Gated on !app.isPackaged via a pure resolveDesktopUninstallUiPreviewMode helper (unit-tested), so it can't fire in a shipped build.
* test(ok-marketing): match the reworded feedback-intake description copy
US-003 reworded the ticket description from 'in-app OpenKnowledge feedback
form' to 'OpenKnowledge feedback surface' (uninstall feedback now arrives from
the desktop main process and the CLI, not just the in-app form), but this
assertion still checked the old wording. Update it to the shipped copy.
* fix(open-knowledge): drop unused exports flagged by knip
pnpm check's knip pass flagged four exports that are only referenced within
their own file — UninstallFeedbackReasonOption, UninstallFeedbackGate,
shouldPromptUninstallFeedback, and DesktopUninstallChecklistItem. Make them
file-local so the quality gate is green.
* fix(open-knowledge): pass uninstall-preview env vars through turbo dev
The dev-only uninstall UI preview reads OK_UNINSTALL_UI_PREVIEW (and the
feedback survey honors OK_FEEDBACK_INTAKE_ORIGIN), but the documented
`pnpm run dev` command routes through `turbo run dev:electron`, which strips
any env var not declared in passThroughEnv. Both vars were silently dropped
before reaching the Electron main process, so the preview never fired. Declare
them so the documented command works.
* polish(open-knowledge): warm the CLI uninstall-feedback ack copy
The post-submit line was terse ('Thanks — that helps.') and both it and the
'Skip' reason used em dashes. Reword the acknowledgement to 'Thank you. We read
every response.' — it reassures a departing user their input isn't a void
(each submission files a triageable ticket), and drop the em dashes ('Skip, I'd
rather not say').
* feat(open-knowledge): restore the data-egress disclosure on the desktop feedback screen
Bring the desktop feedback survey back in line with the CLI survey, which
states where the input goes. Re-add the muted 'What you share is sent to the
OpenKnowledge team.' line under the heading (styled with the shared type scale,
not the old undersized egress rule) and restore aria-describedby so screen
readers announce it. Since the survey collects an optional note + email, naming
the recipient is the honest default.
* refactor(open-knowledge): address review — clarify shared uninstall URL scheme + timeout divergence
Two reviewer 'Consider' items:
- Rename DESKTOP_UNINSTALL_PICKER_SCHEME → DESKTOP_UNINSTALL_SCHEME (+ a comment):
the const is the protocol shared by all three uninstall windows (picker,
feedback, notice), not picker-only, so the old name misled.
- Document why UninstallFeedbackResult carries a 'timeout' member the in-app
FeedbackResult omits: the desktop/CLI path flushes the survey before process
exit and needs the bounded-wait variant; the browser path has no such deadline.
* fix(open-knowledge): print the CLI removal report before the churn survey
Reviewer (pullfrog) flagged that interactive `ok uninstall` prompted 'why are
you leaving?' before the terminal confirmed 'files have been removed'. Defer the
survey via a new UninstallResult.runFeedbackAfterReport thunk that the command
handler awaits AFTER writing the report — so the success confirmation lands
first, then the prompt. Still success-only, still awaited so the POST flushes
before exit. Updates the ordering test to drive the deferred thunk and notes the
sequencing in D15.
* test(open-knowledge): pin the deferred-feedback contract (review follow-up)
Reviewer (tests) noted the runFeedbackAfterReport deferral shifted which tests
exercise the survey closure. Assert the closure is structurally undefined on a
failed removal, and drive it in the SIGINT-absorption test so the interruption
path is actually exercised under the deferred pattern.
GitOrigin-RevId: 33905040b6fb5dcaf3009461b2b435d624b22e50
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Copybara-translated 1 Inkeep OSS change. Rebase-merge this PR so the prepared commit lands directly on public main.